To view this pull requests documentation preview, visit the following URL:

melos.invertase.dev/~526

Documentation is deployed and generated using docs.page.

Another question: should Melos maybe sync the Dart and Flutter version in environment as well?

My two cents:

• Instead of a dedicated separate file, we could include these properties in melos.yaml. It is a feature of melos, so there is no reason to exclude it from the melos settings. At a later point, this could potentially also be a part of pubspec.yaml if request: add melos: section to root pubspec.yaml instead of requiring melos.yaml in Melos v3 #472 is implemented.
• the name shareDependencies makes sense if the dependencies versions are taken from the root package's own dependencies, as suggested by @blaugold in Request: common dependencies #94 (comment), but since your implementation follows another approach, there is no "sharing" anymore. Instead, a more appropriate name could be assignCommonDependencies or similar.
• you could avoid calling https://github.com/invertase/melos/pull/526/files#diff-b5438310509b0fb6f04ef9776c2b9d3ec2a18fe0e0bc7165cdda954148ad9a99R240-R243 if dependenciesUpdated == 0 && devDependenciesUpdated == 0, (which will be the case most of the time, since melos bootstrap is likely to be run more often than common dependencies version are updated).
• There is a mixed usage of the terms "package" and "dependency", which is confusing. I may be wrong, but it looks to me that a "package" refers to a local package managed by melos, while a "dependency" refers to an entry of the "dependencies" and "dev_dependencies" fields. If that is indeed the case, there are several usages of "package" that should be replaced with "dependency":
  • if we wanted to go with a separate file, I guess it should be named "common_dependencies.yaml" rather than "common_packages.yaml"
  • _updatePackages -> _updateDependencies
  • packagesToUpgrade -> dependenciesToUpgrade
  • etc
• the naming of dependenciesUpdated and devDependenciesUpdated sound like booleans to me. I would suggest updatedDependenciesCount and updatedDevDependenciesCount to be more explicit.

But all these are details, compared to the more important design decision of whether we want to share the root package's dependencies or use separate fiels as you have done. As I have already expressed in #94 (comment) , I am more in favour of new fields, but it would be nice to hear @blaugold 's opinion on this design choice.

I would also love to hear what @blaugold has to say as I should have some time over to continue with this PR next week.

Sorry for not responding earlier! 😔 The past few weeks have been really busy.

@lohnn Thanks for working on this!

Another question: should Melos maybe sync the Dart and Flutter version in environment as well?

Great idea. I suspect it's just a question of time until someone asks for this, especially with support for common dependencies.

@vkammerer Thanks for the detailed input. You convinced me that putting the common dependencies into the root pubspec is probably not a good idea. I feel like the next best option is to put them into melos.yaml. Like you, I don't really see a reason not to. We already maintain a JSON schema for melos.yaml, which can be used for autocompletion and validation. Extending that is much easier than introducing a new type of file.

Regarding naming, I would be fine with mirroring that of pubspec.yaml under the bootstrap command options:

command:
  bootstrap:
    environment:
      ...
    dependencies:
      ...
    devDependencies:
      ...

Heh, it's all good! I'm getting married soon and know exactly what you mean. Thankfully I can do some of this job during work hours (wish I could say the same for my own package, but we currently don't use it at work 😅)

Great idea. I suspect it's just a question of time until someone asks for this, especially with support for common dependencies.

Then I'll add it to the TODO list.

Regarding putting the common dependencies in melos.yaml file I only have one gripe. The melos.yaml file is, at least in our project, already quite bloated clocking in at just short of 200 ilnes. Adding all the common dependencies would increase the size of this file even more.

On the flip side, we all use IDEs and can collapse parts of the file if we want to make it more readable, so if you still wish for the common dependencies (and version number) to live in the melos.yaml file, I'll update this PR accordingly :)

Regarding naming, I would be fine with mirroring that of pubspec.yaml under the bootstrap command options:

I imagine you mean it should be:

command:
  bootstrap:
    environment:
      ...
    dependencies: 
      ...
    dev_dependencies: # instead of devDependencies
      ...

@lohnn I would much prefer the simplicity of a few lines added to melos.yaml (which would be visible only when you view that file) than having to deal with yet another config file just for this purpose (which would always be visible when you browse your project in your code editor, which is to say almost all the time).

Moreover, the argument about it belonging to melos responsibilities and therefore belonging to melos.yaml still stands.

Check! I'll take it and update the code accordingly 👍

I believe the implementation of this is now implemented and fully functional. When updating dependencies or environment it will currently print this to show that pubspec has been updated:

  base_project
  └> Updated environment
     Updated 1 dependencies
     Updated 1 dev_dependencies
  screenshot_tests
  └> Updated environment

Untouched packages will not appear at all to not clutter logging unless a change is actually done.

@blaugold

We already maintain a JSON schema for melos.yaml, which can be used for autocompletion and validation. Extending that is much easier than introducing a new type of file.

Where can I find this to update accordingly?

Where can I find this to update accordingly?

The schema is currently maintained here in the repo for the Melos VS Code extension. In the long term, it should be moved here, though (#425).

And regarding tests for this work. I should look into that I guess?

I have now added a test to validate that the bootstrap dependency sync runs as intended, and the PR is now ready to be reviewed.

Ping: I would love for this PR to be reviewed.

@lohnn Sorry for the delay! I'll review it tomorrow.

Thank you for your review and all your comments should now be taken care of :)

I am trying to find this mechanism in the docs website, but there I can only see a dependencies section in bootstrap. Has the common_packages.yaml file being removed?